Skip to content

Fix removing some packages will result dirty data #34709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix removing some packages will result dirty data #34709

wants to merge 1 commit into from
+212 −22

Conversation

lunny
Copy link
Member

@lunny lunny commented Jun 12, 2025

Fix #32830

Inspired by #22810 and replace it.

When deleting alphe, cargo, debian, rpm packages, some extra actions should be taken which was missed. It will result in dirty data and inconsistent. This PR fix it. To avoid recycle dependencies, this is a quick patch. Like #22810 (comment) said, this might need a new design for such more packages types.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 12, 2025
@lunny lunny added type/bug backport/v1.24 This PR should be backported to Gitea 1.24 and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 12, 2025
@github-actions github-actions bot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code and removed type/bug backport/v1.24 This PR should be backported to Gitea 1.24 labels Jun 12, 2025
@lunny lunny added type/bug backport/v1.24 This PR should be backported to Gitea 1.24 labels Jun 12, 2025
@lunny lunny requested a review from KN4CK3R June 13, 2025 06:32
@lunny lunny mentioned this pull request Jun 17, 2025
Closed
@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 17, 2025

I don't think this is cleaner than my PR. With this change some package types must call the remove method from the router, other types must call the remove method from the service.

@lunny
Copy link
Member Author

lunny commented Jun 18, 2025 •
edited
Loading

I don't think this is cleaner than my PR. With this change some package types must call the remove method from the router, other types must call the remove method from the service.

Your implementation will still have a chance to fail with dirty data. The alpine_service.BuildAllRepositoryFiles(ctx, pd.Owner.ID) should be invoked in the same transaction inside RemovePackageVersion and the notification should not be invoked before all cleanup succeed. Currently, only routers invoke these functions, a new service package could be introduced if it will be invoked from command or other packages. As a quick patch, I think it's enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KN4CK3R KN4CK3R Awaiting requested review from KN4CK3R

Assignees
No one assigned
Labels
backport/v1.24 This PR should be backported to Gitea 1.24 lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deleting a debian package does not remove it from the apt Packages list that clients see
3 participants
@lunny @KN4CK3R @GiteaBot